Skip to content

Poc doctrine migrations#2827

Closed
LukasReschke wants to merge 7 commits into
masterfrom
poc-doctrine-migrations
Closed

Poc doctrine migrations#2827
LukasReschke wants to merge 7 commits into
masterfrom
poc-doctrine-migrations

Conversation

@LukasReschke

@LukasReschke LukasReschke commented Dec 22, 2016

Copy link
Copy Markdown
Member

From owncloud/core#14851 – needs extensive testing …

Requires nextcloud/3rdparty#24

DeepDiver1975 and others added 4 commits December 22, 2016 14:16
Signed-off-by: Lukas Reschke <lukas@statuscode.ch>
Signed-off-by: Lukas Reschke <lukas@statuscode.ch>
Signed-off-by: Lukas Reschke <lukas@statuscode.ch>
Signed-off-by: Lukas Reschke <lukas@statuscode.ch>
@mention-bot

Copy link
Copy Markdown

@LukasReschke, thanks for your PR! By analyzing the history of the files in this pull request, we identified @nickvergessen, @bartv2 and @georgehrke to be potential reviewers.


/**
* @param \OCP\IConfig $config
*/

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

PHPDoc should be adjusted.


/**
* @param \OCP\IConfig $config
*/

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

PHPDoc should be adjusted


/**
* @param \OCP\IConfig $config
*/

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

PHPDoc should be adjusted


/**
* @param \OCP\IConfig $config
*/

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

PHPDoc should be adjusted

*/
public function loadSchemaFromFile($file) {
$schema = new \Doctrine\DBAL\Schema\Schema();
public function loadSchemaFromFile($file, $schema = null) {

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

$schema is not in PHPDoc

Comment thread lib/private/Setup/AbstractDatabase.php Outdated
<<<<<<< HEAD
use OC\AllConfig;
use OC\DB\ConnectionFactory;
=======

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Merge fuckup :)

A bool is not always a bool...

Signed-off-by: Lukas Reschke <lukas@statuscode.ch>
@LukasReschke
LukasReschke force-pushed the poc-doctrine-migrations branch from 5ca9b00 to 545b5d2 Compare December 22, 2016 13:25
* @param \OCP\IConfig $config
*/
public function __construct(IConfig $config, Connection $connection) {
$this->config = $config;

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Unused

Signed-off-by: Lukas Reschke <lukas@statuscode.ch>
Signed-off-by: Lukas Reschke <lukas@statuscode.ch>
@LukasReschke

Copy link
Copy Markdown
Member Author

Also there is owncloud/activity#546 which uses this approach.

FYI @nickvergessen

@codecov-io

Copy link
Copy Markdown

Current coverage is 53.50% (diff: 22.00%)

Merging #2827 into master will decrease coverage by 0.01%

@@             master      #2827   diff @@
==========================================
  Files          1298       1308    +10   
  Lines         80337      80666   +329   
  Methods        7957       8021    +64   
  Messages          0          0          
  Branches       1242       1242          
==========================================
+ Hits          42996      43164   +168   
- Misses        37341      37502   +161   
  Partials          0          0          
Diff Coverage File Path
0% lib/private/Setup/AbstractDatabase.php
0% new core/Command/Db/Migrations/MigrateCommand.php
0% lib/private/DB/OracleConnection.php
0% new core/Command/Db/Migrations/StatusCommand.php
0% new core/Command/Db/Migrations/GenerateCommand.php
0% lib/private/Updater.php
0% lib/private/legacy/app.php
0% new core/Command/Db/Migrations/ExecuteCommand.php
0% lib/private/Setup.php
0% core/register_command.php

Review all 14 files changed

Powered by Codecov. Last update 7f8d22e...980f472

@MorrisJobke

Copy link
Copy Markdown
Member

From owncloud/core#14851 – needs extensive testing …

Yep :) I would say we should test this with a first real world example for those migrations and get this in as fast as possible. Then do all our migrations with this tool.

@MorrisJobke MorrisJobke left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good so far - 3rdparty needs to be fixed

@LukasReschke

Copy link
Copy Markdown
Member Author

Closing this as upstream went with another approach and we should reconsider on how we want to proceed here. We can reopen at any time anyhow…

@LukasReschke
LukasReschke deleted the poc-doctrine-migrations branch February 20, 2017 20:40
@MorrisJobke

Copy link
Copy Markdown
Member

Closing this as upstream went with another approach and we should reconsider on how we want to proceed here. We can reopen at any time anyhow…

I would like to go the doctrine migrations approach.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

2. developing Work in progress

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants